Skip to content

refactor intrusive list#150

Open
wokron wants to merge 1 commit into
masterfrom
refactor-intrusive-list
Open

refactor intrusive list#150
wokron wants to merge 1 commit into
masterfrom
refactor-intrusive-list

Conversation

@wokron

@wokron wokron commented Jun 30, 2026

Copy link
Copy Markdown
Member

PR Type

Enhancement


Description

  • Refactored intrusive list to separate implementation from type-safe wrapper

  • Extracted SingleLinkListImpl and DoubleLinkListImpl as non-template base classes

  • Moved template logic into new IntrusiveSingleList and IntrusiveDoubleList wrappers

  • Updated splice method to clear source list after moving entries


Diagram Walkthrough

flowchart LR
  A["IntrusiveSingleList<T, Member>"] --> B["SingleLinkListImpl"]
  C["IntrusiveDoubleList<T, Member>"] --> D["DoubleLinkListImpl"]
  B -- "push_back, pop_front, splice" --> E["SingleLinkEntry"]
  D -- "push_back, pop_front, remove" --> F["DoubleLinkEntry"]
Loading

File Walkthrough

Relevant files
Enhancement
intrusive.hpp
Refactored intrusive list into impl and wrapper                   

include/condy/detail/intrusive.hpp

  • Extracted SingleLinkListImpl and DoubleLinkListImpl as non-template
    classes
  • Created new template wrappers IntrusiveSingleList and
    IntrusiveDoubleList
  • Updated splice to clear source list after moving entries
  • Changed push_back, pop_front, and remove to accept raw entry pointers
+89/-27 

@wokron

wokron commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

/review

@github-actions

github-actions Bot commented Jun 30, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 61032fa)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Missing splice

The original IntrusiveSingleList had a splice method that was removed in the refactor. The new SingleLinkListImpl does not expose a splice operation, and the wrapper IntrusiveSingleList does not provide one either. If any caller relied on splice, this change will break compilation or require rework. Verify that splice is no longer needed or add it to the new implementation.

template <typename T, SingleLinkEntry T::*Member> class IntrusiveSingleList {
public:
    IntrusiveSingleList() = default;
    IntrusiveSingleList(IntrusiveSingleList &&other) noexcept
        : impl_(std::move(other.impl_)) {}
    IntrusiveSingleList &operator=(IntrusiveSingleList &&other) noexcept {
        if (this != &other) {
            impl_ = std::move(other.impl_);
        }
        return *this;
    }

    CONDY_DELETE_COPY(IntrusiveSingleList);

public:
    void push_back(T *item) noexcept { impl_.push_back(&(item->*Member)); }

    void push_back(IntrusiveSingleList other) noexcept {
        impl_.push_back(other.impl_);
    }

    bool empty() const noexcept { return impl_.empty(); }

    T *pop_front() noexcept {
        SingleLinkEntry *entry = impl_.pop_front();
        if (!entry) {
            return nullptr;
        }
        return container_of(Member, entry);
    }

private:
    SingleLinkListImpl impl_;
};
Missing splice

The original IntrusiveDoubleList had a splice method that is absent in the refactored DoubleLinkListImpl and its wrapper IntrusiveDoubleList. If any existing code uses splice, it will fail to compile. Confirm that splice is intentionally removed or re-add it.

template <typename T, DoubleLinkEntry T::*Member> class IntrusiveDoubleList {
public:
    IntrusiveDoubleList() = default;
    IntrusiveDoubleList(IntrusiveDoubleList &&other) noexcept
        : impl_(std::move(other.impl_)) {}
    IntrusiveDoubleList &operator=(IntrusiveDoubleList &&other) noexcept {
        if (this != &other) {
            impl_ = std::move(other.impl_);
        }
        return *this;
    }

    CONDY_DELETE_COPY(IntrusiveDoubleList);

public:
    void push_back(T *item) noexcept { impl_.push_back(&(item->*Member)); }

    bool empty() const noexcept { return impl_.empty(); }

    T *pop_front() noexcept {
        DoubleLinkEntry *entry = impl_.pop_front();
        if (!entry) {
            return nullptr;
        }
        return container_of(Member, entry);
    }

    bool remove(T *item) noexcept { return impl_.remove(&(item->*Member)); }

private:
    DoubleLinkListImpl impl_;
};

@github-actions

Copy link
Copy Markdown

PR Description updated to latest commit (cfff96f)

@wokron wokron force-pushed the refactor-intrusive-list branch from cfff96f to 61032fa Compare June 30, 2026 08:42
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 61032fa

@wokron wokron marked this pull request as ready for review June 30, 2026 09:07
@wokron wokron force-pushed the refactor-intrusive-list branch from 61032fa to c4d2cbf Compare June 30, 2026 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant